Code Review: PR #134

Общее впечатление

PR хорошо структурирован. Whitelist в ProductDataService — важное улучшение безопасности (старый код передавал raw input в fromArray() без фильтрации). Lifecycle инлайн-редактирования продуман: double-click → edit → blur/Enter saves, Escape cancels, unchanged values не шлют запрос.

Баг: autofocus не работает на динамически вставленных элементах

CategoryProductsGrid.vue:1029, 1040

<InputText ... autofocus />
<InputNumber ... autofocus />

HTML-атрибут autofocus работает только при загрузке страницы. При динамической вставке элемента через v-if он не получит фокус. Пользователь двойным кликом откроет редактор, но курсор в инпуте не окажется — придётся кликать ещё раз.

Фикс: использовать Vue-директиву или ref + nextTick:

// В startInlineEdit(), после установки editingCell:
nextTick(() => {
  const input = document.querySelector('.inline-edit-cell input')
  if (input) input.focus()
})

Или кастомная директива v-focus.

Баг: двойной вызов saveInlineEdit (Enter + blur)

CategoryProductsGrid.vue:1030-1031

@blur="saveInlineEdit(product, column)"
@keydown.enter="saveInlineEdit(product, column)"

Когда пользователь нажимает Enter:

1. @keydown.enter вызывает saveInlineEdit() → clearInlineEditState() → editingCell = null
2. Vue убирает <InputText> из DOM (условие v-else-if больше не true)
3. Удаление из DOM вызывает blur → второй вызов saveInlineEdit()

Guard в начале saveInlineEdit (if (!editingCell.value || ...)) защищает от повторного API-вызова — второй вызов вернётся рано. Но при ошибке API (catch-блок делает return без clearInlineEditState), editingCell остаётся, и blur после Enter снова попытается сохранить.

Рекомендация: в @keydown.enter сначала сделать $event.target.blur() и убрать отдельный @keydown.enter handler, либо добавить флаг isSaving чтобы исключить параллельные вызовы.

Отсутствие MODX-событий при publish/unpublish

ProductDataService.php:501-512 — applyPublishedToResource()

Метод напрямую меняет published, publishedon, publishedby через set() + save(). MODX-события OnDocPublished / OnDocUnPublished не вызываются. Если плагины зависят от этих событий (кеширование, индексация, уведомления), они не сработают.

Рекомендация: Использовать $product->set('published', ...) + $product->save() с ручным вызовом событий, или как минимум оставить // TODO комментарий что события пропущены осознанно.

editor_options в whitelist но не используется

GridConfigService.php:190 — ключ editor_options добавлен в $configKeys. В лексиконах есть editor_type_select и editor_options. Но в UI select-редактор не реализован — editorTypeOptions содержит только text/number.

Не баг, но мёртвый конфиг-ключ. Если select планируется позже — стоит добавить // TODO: select editor комментарий.

Нет индикации загрузки при сохранении

CategoryProductsGrid.vue:420-428 — saveInlineEdit() делает await request.put(...) без визуального feedback'а. При медленном соединении пользователь может подумать что ничего не происходит и кликнуть снова.

Рекомендация (не блокер): добавить кратковременный loading-индикатор или disabled-состояние на ячейку.

Что сделано хорошо

• Whitelist $allowedUpdateFields — критическое улучшение безопасности. Старый код передавал сырой input напрямую в fromArray()
• Разделение resource/productData полей — published обновляется через отдельный метод с правильной обработкой publishedon/publishedby
• isInlineValueUnchanged() — грамотная оптимизация, пропускает API-вызов и тост если значение не изменилось
• Guard в saveInlineEdit — проверка editingCell в начале защищает от гонок
• Лексиконы на двух языках — ru + en
• isCategoryProductsGrid — inline edit показывается только для релевантного грида

Резюме

Code Review: inline edit for category products table

+367 / -16 | 6 файлов | 2 коммита

Общая оценка

Фича добавляет инлайн-редактирование в таблице товаров категории (двойной клик по ячейке). Включает: UI в гриде, конфигурацию editable/editor_type в настройках колонок, whitelist-фильтрацию полей на бэкенде, обработку published через ивенты MODX.

1. Безопасность: whitelist — хорошо, но есть пробел

ProductDataService.php — до этого PR метод updateProductData() делал $productData->fromArray($data) без фильтрации. Теперь есть $allowedUpdateFields + array_intersect_key(). Это правильно и важно.

Однако: нет проверки прав на редактирование. Метод вызывается через /api/mgr/product-data/{id} — менеджерский API, доступ ограничен авторизацией. Но пользователь с view без save_document сможет редактировать. Стоит проверять $this->modx->hasPermission('save_document') или $product->checkPolicy('save').

2. Порядок сохранения: race condition между resource и productData

// Сначала сохраняется resource (published)
$updatedResource = $this->applyPublishedToResource($product, $published);

// Потом productData
if (!$productData->save()) {
    return null;  // resource уже сохранён!
}

Если productData->save() упадёт, published уже изменён и сохранён. Это рассинхронизация — юзер получит ошибку, но published фактически изменился. Нужно сначала сохранять productData, потом resource. Или оба в транзакции.

3. Ответ API не используется на фронте

await request.put(`/api/mgr/product-data/${product.id}`, { [column.name]: value })
product[column.name] = value  // локальное обновление нормализованным значением

Ответ сервера игнорируется. Если бэкенд трансформирует значение (округление цены, приведение типа), локальное состояние будет отличаться от серверного. Лучше:

const result = await request.put(...)
Object.assign(product, result.data)  // или хотя бы product[column.name] = result.data[column.name]

4. DOM-запрос для фокуса — хрупко

nextTick(() => {
  const input = document.querySelector('.inline-edit-cell input')
  if (input) input.focus()
})

Глобальный document.querySelector найдёт первый .inline-edit-cell input на странице. Для PrimeVue InputNumber реальный <input> может быть вложен глубже. Надёжнее — ref на компоненте:

<InputText ref="inlineInput" ... />

nextTick(() => inlineInput.value?.$el?.focus())

5. @blur на InputNumber — потенциальная проблема PrimeVue

PrimeVue InputNumber оборачивает <input> в <span>. Событие @blur на компоненте может не пробрасываться корректно с внутреннего <input>. Нужно проверить, что save действительно вызывается при клике вне ячейки. Если нет — нужен @blur с .native или обработка через onBlur prop.

Аналогично @keydown.escape — может не долетать от вложенного input до компонента.

6. editor_options и editor_type_select — мёртвый код

'editor_options', // TODO: select editor - not yet implemented in UI

Лексикон editor_type_select добавлен, editor_options сохраняется в конфиге, но UI для select-редактора нет. Это нормально для итеративной разработки, но стоит либо убрать до реализации, либо добавить issue. Сейчас это dead code + TODO в продакшн-коде.

7. Нет валидации значений на бэкенде

updateProductData() принимает данные и ставит через fromArray(). xPDO не валидирует значения — можно установить отрицательную цену, нечисловой stock и т.д. Минимальная валидация:

• price / old_price >= 0
• stock — integer
• weight >= 0

8. Мелочи

• (int)(bool)$resourceData['published'] — двойное приведение. (int)$resourceData['published'] достаточно если нужно 0/1. Или $resourceData['published'] ? 1 : 0 для ясности.
• Лексикон inline_edit_hint — длинный текст в подсказке. Проверить что не ломает layout при узких экранах.
• cursor: text на .editable-cell — подсказывает юзеру, но визуально не отличает editable от non-editable ячеек. Лёгкий hover-эффект (background) был бы полезнее.

Итого

Рекомендация: исправить пункты 2, 3, 4, добавить проверку permissions (п.1) и минимальную валидацию (п.7).